New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MYST-413 Fix broken Alpine image #197
Conversation
|
||
# Install application | ||
COPY bin/client_docker/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh | ||
ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From docker perspective it's not very efficient to put executable AND it's args into entry point, as it will be very hard to override params later. It should be like this:
ENTRYPOINT ["your exec and args which must be always present and never change"]
CMD["default additional args which can be customized when using docker run style"]
those could be tequila.port , address , discovery address etc.
Do we really even need docker-entrypoint.sh ? it hides important stuff, also uses env vars internally and it's hard to track what is going on
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good ideas, will depreciate Docker environments variables in upcoming PRs
docker-compose.yml
Outdated
cap_add: | ||
- MKNOD | ||
- NET_ADMIN | ||
networks: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to force use default network. Each docker compose creates and maintains it's own network automatically
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed network injection
bin/client_build
Outdated
@@ -13,7 +13,7 @@ | |||
#> GOOS=windows GOARCH=amd64 bin/client_build | |||
# | |||
# Check if program has dynamic libraries: | |||
#> readelf -d /build/client/mysterium_client | |||
#> readelf -d build/client/mysterium_client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no readelf on osx by default. We should add help notice how to get it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far
No description provided.